Skip to content

Sync: codex managed launch S5 (DEV-0006/0008 closure), #582 review hardening, fork-watch race fix - #588

Merged
31 commits merged into
mainfrom
sync/s5-managed-launch-and-hardening
Jul 30, 2026
Merged

Sync: codex managed launch S5 (DEV-0006/0008 closure), #582 review hardening, fork-watch race fix#588
31 commits merged into
mainfrom
sync/s5-managed-launch-and-hardening

Conversation

@danshapiro

Copy link
Copy Markdown
Owner

Aligns local main work with origin. Contents:

Verification (all at tip): cargo fmt/clippy/check clean; cargo test --workspace green except the documented pre-existing pane_ledger lock flake (DEFLAKE f3wp); client 4702 + server 4769 vitest tests passing; tsc clean.

Generated with Amplifier

Co-Authored-By: Amplifier [email protected]

danshapiro and others added 30 commits July 30, 2026 00:35
…estigation)

- S1-S4 confirmed landed dark (flag default OFF, G-X0 still live golden)
- Slice 5 rewritten: original scope pre-empted by the disk-based identity
  lane (rollout locator + pane ledger + freshell-activity); revised S5 =
  drain proxy events into existing tails (S5.a), locator arbitration (S5.b),
  enforce require_candidate_persistence (S5.c), structural prereqs (S5.d),
  flag flip + golden swap + record closure under D-C-REVISIT (S5.e)
- §7 anchors re-anchored to 2026-07-30 main; new §8 reconciliation with
  discrepancy log (D-01 stale DEV-0008 record text, D-02 fence overtaken,
  D-03 locator-vs-candidate precedence OPEN)

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
Stage-2 validation of 28 load-bearing assumptions (20 verified, 4 falsified,
4 accepted residuals). Plan fixes:
- Task 7: suppress the REST door's codex-locator arming too (A10 falsified:
  arm_locators_for_fresh_pane is a third arm path)
- Task 4: proxy-lane dedupe hardened (A11): TurnStarted disarms the stale
  proxy swallow; BEL-initiated clears arm swallow_next_proxy_complete; +2 tests
- Task 6: legacy bind-predicate parity filter (non-empty id + absolute rollout
  path, A25/A9); unconditional sink install before serve (A3); A22/A7 notes
- Task 1: definitive --features real-transport invocation (A26); legacy gate
  nuances (A28)
- Task 8: canonical const is CODEX_MANAGED_REMOTE_CONFIG_ARGS; no freshagent
  argv-pin test exists - explicit pre/post parity check required (A23/A26)
- Task 9: no AlreadyExists->409 copy; pin the two accepted deltas (A15/A16)
- Preamble: worktree needs npm ci for freshagent suite; known WSL2 flake (A27)

Ledger: .worktrees/.the-usual-logs/codex-managed-launch-s5/load-bearing-ledger.md
…teration 1)

Two blocking defects found by independent cross-model review:

1. Task 1's candidate-persistence gate would break the existing
   remote_proxy_relay.rs fork-rewrite tests (all relay tests construct
   the proxy with require_candidate_persistence == true) while the plan
   claimed pre-existing tests stay green. Added Step 3i scheduling the
   relay-suite remediation (release the gate in the two thread/fork
   rewrite tests), listed the file in Task 1 Files and the File
   Structure map, and corrected Step 4's expected-pass rationale.

2. Tasks 2, 3, and 9 TDD red/green commands omitted
   --features real-transport, so 'Expected: FAIL to compile' could not
   fail (empty test binary, exit 0) and 'Expected: PASS' was vacuous —
   launch_lifecycle (lib.rs:50-51) and tests/launch_lifecycle.rs (:13)
   only compile behind that default-off feature (ledger A26). Added the
   flag and the rationale to all six command sites.
…teration 2)

- Task 4: note_proxy_turn_started now disarms swallow_next_proxy_complete
  (its own disarm test was unpassable); add the missing BEL-clear arming
  edit (note_output's consume_turn_complete_signal site, ledger A11) and
  make bel_clear_swallows_the_late_proxy_echo non-vacuous via a queued
  follow-up submit; test snippets use the verified &str call shapes.
- Task 1: gate implementation now mirrors all three A28 legacy nuances
  (push-then-fail hold-queue overflow, cumulative held-bytes cap,
  candidate_capture_timeout on ANY initial-capture failure) with three
  new/extended candidate_gate.rs tests pinning them.
- Task 10/13: D-C-REVISIT doc comments keep RESOLVED on the marker line
  so Task 13's 'grep -v RESOLVED' closure gate passes as written.
…ew (iteration 3)

Move the 'pub mod codex_proxy_route;' lib.rs declaration from Task 6 Step 3
into Step 1: an undeclared src/*.rs file is not part of the crate graph, so
Step 2's 'cargo test -p freshell-ws codex_proxy_route' would compile the
unchanged crate, run 0 tests, and exit 0 — making the required 'Expected:
FAIL to compile' unachievable (same vacuous-pass trap class as ledger A26).
With the declaration landing alongside the tests, the RED step fails for the
right reason (router functions not yet written). Step 3's paragraph updated
to note the declaration already landed in Step 1.
…mit no longer held across planning (S5.e precondition)
… test suites OFF (DEV-0006 S5.e)

Beyond the four suites the spec names (codex_fork_rebind, codex_locator_activity, codex_session_ref_resume, codex_candidate_inert), the sweep/--ignored net found two more codex-spawning tests that needed the same set_var(.., "0") pin:

- crates/freshell-freshagent/src/terminal_tabs.rs::send_keys_enter_feeds_codex_locator (REST fake-codex create; managed path suppresses the locator arm)
- crates/freshell-ws/tests/codex_managed_launch_e2e.rs Phase 1 (the explicit flag-OFF control; post-flip remove_var means ON)
…eating tests missed by the S5.e flag flip

Fresh-eyes review of the S5 delta found that 6a8733a (default the
managed-launch flag ON) did not pin the plain-CLI suites completely:
three ws integration tests (pane_ledger_triggers: fresh-pending marker
+ resume-binding retire; pane_ledger_restore: inventory stamping) and
one freshagent REST unit test (create_codex_tab_accepts_session_ref_
and_derives_resume_args) create mode:"codex" terminals with the flag
unset, so post-flip they routed through the managed-launch planner and
spawned the real codex binary from PATH — failing on codex-less hosts
(PTY_SPAWN_FAILED / 5x226s retry burn) and leaking real app-server
sidecars against ~/.codex on dev hosts.

Pin each test OFF with the exact per-test convention 6a8733a used
elsewhere (set_var to "0" + DEV-0006 S5.e comment), which also makes
the flip commit's and DEVIATIONS.md's 'plain-CLI suites pinned OFF'
claim true. Verified: all three suites now pass with
CODEX_CMD=/nonexistent-codex-binary.
Post-merge code-review pass authored 2026-07-29/30 by three misdirected
code-review skill forks; content audited and adopted deliberately:

- dedupe env parsers into crate::env_parse (freshell-ws)
- extract fail_json_code_retry_after; use in spawn_gate_error_response
- single broadcast_frame/broadcast_settled_frame home + SETTLE_REASON_CANCELLED
- fix two real leaks: auto-resume attempts map growth on terminal exit,
  stranded auto_resume_cancels entries (+ stale-entry sweep)
- move CrashTrace to shared/session-contract.ts as zod schema with
  sanitizeRestoreError/sanitizeCrashTrace helpers; consume in client stores
- make recordAutoResumeSettled redelivery-idempotent without materializing
  missing entries; tests updated

Verified: cargo check clean (3434 files), client tsc clean, 232 client
tests passing; authoring forks additionally verified fmt/clippy clean,
330 Rust lib tests, 183 client tests.

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
…ignore scratch

- commit 5 plan docs (net08 audit, remote-access networking x3,
  rust-resolve-parity spec) per repo convention
- commit .discovery/ (Parallax DEV-0006 investigation artifacts referenced
  by docs/plans/2026-07-19-dev0006-codex-launch-planning-spec.md §7/§8,
  plus code-review skill hardening proposal)
- gitignore agent scratch: .ai/, .amp-council/, .verify-vantages.env
- delete empty debug artifact sse.out

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
…V-0006/DEV-0008)

Flips FRESHELL_CODEX_MANAGED_LAUNCH default ON: codex terminal panes launch
managed (app-server sidecar + remote proxy, --remote 4-tuple,
features.apps=false), with candidate-persistence gating, proxy event drain
into the existing identity/activity tails, third activity dedupe lane,
spawn-helper unification, G-X0 retired / G-X1-X2 promoted, and DEV-0006 +
DEV-0008 deviation records closed.

Per docs/plans/2026-07-19-dev0006-codex-launch-planning-spec.md (revised
Slice 5, §8 reconciliation) and the worktree plan
docs/plans/2026-07-30-codex-managed-launch-s5.md. 22 commits, 13/13 tasks,
independent delta review PASSED, whole-branch review APPROVED FOR MERGE.

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
…rget verification)

Incident (2026-07-30): three forks of the bundled `code-review` skill —
misloaded by the-usual recipe step-runners in place of the read-only
`requesting-code-review` template skill — woke up with empty $ARGUMENTS
and this main checkout as inherited cwd, diffed the dirty main tree, and
applied 21 unwanted "fixes" to it.

This workspace override shadows the cached bundle skill (first-match-wins)
and adds the guardrails proposed in
.discovery/code-review-skill-hardening.md (R2):

- Phase 0 "Verify the Target": parse $ARGUMENTS for an explicit repo path
  and expected branch; echo `git rev-parse --show-toplevel`, the current
  branch, and `git status --porcelain`; REFUSE (report-only, zero edits,
  no review agents) on branch mismatch, or when no explicit target was
  given and the cwd repo is on main/master with uncommitted changes the
  session cannot attribute to itself.
- Phase 1: the incoherent fork fallback ("files you edited earlier in
  this conversation" — always empty in a fork) is replaced with: review
  files named in $ARGUMENTS, else report "nothing to review" and stop.
- Phase 3: fixes are applied ONLY when Phase 0 verified the target;
  otherwise findings go to a report file with ZERO edits. Invariant:
  never write into a dirty tree whose changes you cannot attribute.

The skill still works as a fork skill when correctly targeted
(explicit path + matching branch in $ARGUMENTS).

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
…entity (#584), deck visual tweaks (#585)

Conflict resolution (crates/freshell-freshagent/src/terminal_tabs.rs,
settle_gated_create): kept BOTH sides, ordered spawn-gate acquire (D-C-R,
ours) BEFORE the PIN2 claude prespawn durable binding (theirs) — a gate
rejection must not leave a stale prespawn ledger row, so the durable write
happens only after a permit is secured.

Post-merge semantic fix: pinned FRESHELL_CODEX_MANAGED_LAUNCH=0 in
rest_locator_identity.rs::rest_created_codex_pane_binds_identity_row_and_ledger
(new in #584, written pre-flip; exercises the plain-CLI codex path with a
sleeper fake that cannot serve an app-server sidecar).

Pre-merge analysis recorded no redundancy between the #582 review-hardening
(d9705fd) and the remote delta; #586 touches resume-resolve-contract.ts,
not session-contract.ts — CrashTrace work composes cleanly.

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
…existing race)

drain_and_associate registered watch_fork AFTER adopt_codex_identity
broadcast terminal.session.associated — the client's cue that the pane is
bound. A client driving the fork dance immediately after that frame could
race the watch registration two ways: (1) note_fork_submit before
watch_fork => no fork window ever opens; (2) watch_fork's known_files
snapshot completing after the fork rollout write => the new rollout
permanently excluded as a candidate. Either way: no rebind, 10s timeout.

Move watch_fork registration BEFORE adopt_codex_identity so the watch and
snapshot provably precede the client-visible broadcast; on adoption
refusal, locator.disarm() drops the eager watch, restoring the refused
pane's prior end state.

Predates the 2026-07-30 merge (identical failure rates at baseline
4baf8e5 under load: 4/8 vs 3/8); surfaced by workspace test-load growth.
Verified: codex_fork_rebind 8/8 under 32-core load (was 4/15 failing),
3/3 consecutive full-suite passes, codex_association lib tests 8/8,
cargo check clean.

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
…on contention)

Four pillars: P1 planning off-permit, P2 restore-class queueing (no
wall-clock death), P3 restoreProgress protocol + client placeholder, P4
auto-resume hub owns restore-retry. Taxonomy classifies 11 failure paths;
3 violate the mandate today. Recommended first slice: P1+P2 (server-only,
protocol-invisible) before the next server restart with many codex tabs.

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
@danshapiro danshapiro closed this pull request by merging all changes into main in 39010cb Jul 30, 2026
@danshapiro
danshapiro deleted the sync/s5-managed-launch-and-hardening branch July 30, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant